-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[PAC][ThinLTO] Fix auth key for GOT entries of function symbols #131467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Symtab is first filled with the data from the bitcode file, and all symbols except TLS ones are `STT_NOTYPE`. Since auth key for a signed GOT entry depends on the symbol type being `STT_FUNC` or not, we need to update the symtab after the bitcode is compiled to an ELF object and update symbol types for function symbols. This patch implements the described behavior.
eacd62a to
479f585
Compare
|
@llvm/pr-subscribers-lld Author: Daniil Kovalev (kovdan01) ChangesSymtab is first filled with the data from the bitcode file, and all symbols except TLS ones are Full diff: https://github.com/llvm/llvm-project/pull/131467.diff 2 Files Affected:
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 18f9fed0d08e2..434f17786c861 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2618,6 +2618,13 @@ void LinkerDriver::compileBitcodeFiles(bool skipLinkedOutput) {
auto *obj = cast<ObjFile<ELFT>>(file.get());
obj->parse(/*ignoreComdats=*/true);
+ for (typename ELFT::Sym elfSym : obj->template getGlobalELFSyms<ELFT>()) {
+ StringRef elfSymName = check(elfSym.getName(obj->getStringTable()));
+ if (Symbol *sym = ctx.symtab->find(elfSymName))
+ if (sym->type == STT_NOTYPE)
+ sym->type = elfSym.getType();
+ }
+
// For defined symbols in non-relocatable output,
// compute isExported and parse '@'.
if (!ctx.arg.relocatable)
diff --git a/lld/test/ELF/lto/aarch64-pac-got-func.ll b/lld/test/ELF/lto/aarch64-pac-got-func.ll
new file mode 100644
index 0000000000000..e42f78d30adba
--- /dev/null
+++ b/lld/test/ELF/lto/aarch64-pac-got-func.ll
@@ -0,0 +1,38 @@
+; REQUIRES: aarch64
+
+; RUN: llvm-as %s -o %t.o
+; RUN: ld.lld %t.o -shared -o %t
+; RUN: llvm-readelf -r -x.got %t | FileCheck %s
+
+; CHECK: Relocation section '.rela.dyn' at offset 0x2a8 contains 2 entries:
+; CHECK-NEXT: Offset Info Type Symbol's Value Symbol's Name + Addend
+; CHECK-NEXT: 00000000000203d8 0000000100000412 R_AARCH64_AUTH_GLOB_DAT 0000000000000000 foo + 0
+; CHECK-NEXT: 00000000000203e0 0000000200000412 R_AARCH64_AUTH_GLOB_DAT 0000000000000000 var + 0
+
+; CHECK: Hex dump of section '.got':
+; CHECK-NEXT: 0x000203d8 00000000 00000080 00000000 000000a0
+;; ^^ 0b10000000 bit 63 address diversity = true, bits 61..60 key = IA
+;; ^^ 0b10100000 bit 63 address diversity = true, bits 61..60 key = DA
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+@var = external global ptr
+
+declare void @foo()
+
+define void @bar() #0 {
+entry:
+ store ptr ptrauth (ptr @foo, i32 0), ptr @var
+ ret void
+}
+
+define void @_start() {
+entry:
+ ret void
+}
+
+attributes #0 = {"target-features"="+pauth"}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 8, !"ptrauth-elf-got", i32 1}
|
|
@llvm/pr-subscribers-lld-elf Author: Daniil Kovalev (kovdan01) ChangesSymtab is first filled with the data from the bitcode file, and all symbols except TLS ones are Full diff: https://github.com/llvm/llvm-project/pull/131467.diff 2 Files Affected:
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 18f9fed0d08e2..434f17786c861 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2618,6 +2618,13 @@ void LinkerDriver::compileBitcodeFiles(bool skipLinkedOutput) {
auto *obj = cast<ObjFile<ELFT>>(file.get());
obj->parse(/*ignoreComdats=*/true);
+ for (typename ELFT::Sym elfSym : obj->template getGlobalELFSyms<ELFT>()) {
+ StringRef elfSymName = check(elfSym.getName(obj->getStringTable()));
+ if (Symbol *sym = ctx.symtab->find(elfSymName))
+ if (sym->type == STT_NOTYPE)
+ sym->type = elfSym.getType();
+ }
+
// For defined symbols in non-relocatable output,
// compute isExported and parse '@'.
if (!ctx.arg.relocatable)
diff --git a/lld/test/ELF/lto/aarch64-pac-got-func.ll b/lld/test/ELF/lto/aarch64-pac-got-func.ll
new file mode 100644
index 0000000000000..e42f78d30adba
--- /dev/null
+++ b/lld/test/ELF/lto/aarch64-pac-got-func.ll
@@ -0,0 +1,38 @@
+; REQUIRES: aarch64
+
+; RUN: llvm-as %s -o %t.o
+; RUN: ld.lld %t.o -shared -o %t
+; RUN: llvm-readelf -r -x.got %t | FileCheck %s
+
+; CHECK: Relocation section '.rela.dyn' at offset 0x2a8 contains 2 entries:
+; CHECK-NEXT: Offset Info Type Symbol's Value Symbol's Name + Addend
+; CHECK-NEXT: 00000000000203d8 0000000100000412 R_AARCH64_AUTH_GLOB_DAT 0000000000000000 foo + 0
+; CHECK-NEXT: 00000000000203e0 0000000200000412 R_AARCH64_AUTH_GLOB_DAT 0000000000000000 var + 0
+
+; CHECK: Hex dump of section '.got':
+; CHECK-NEXT: 0x000203d8 00000000 00000080 00000000 000000a0
+;; ^^ 0b10000000 bit 63 address diversity = true, bits 61..60 key = IA
+;; ^^ 0b10100000 bit 63 address diversity = true, bits 61..60 key = DA
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+@var = external global ptr
+
+declare void @foo()
+
+define void @bar() #0 {
+entry:
+ store ptr ptrauth (ptr @foo, i32 0), ptr @var
+ ret void
+}
+
+define void @_start() {
+entry:
+ ret void
+}
+
+attributes #0 = {"target-features"="+pauth"}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 8, !"ptrauth-elf-got", i32 1}
|
|
Would be glad to see feedback from anyone interested |
|
@MaskRay Would be glad to see your feedback on the changes |
1 similar comment
|
@MaskRay Would be glad to see your feedback on the changes |
|
This is somewhat unusual, as we typically don’t track the type of undefined symbols. The subject could likely be clarified to refer specifically to undefined symbols. I see how this happens. When the module flag "ptrauth-elf-got" is set, llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp sets the function symbol |
|
|
||
| define void @bar() #0 { | ||
| entry: | ||
| store ptr ptrauth (ptr @foo, i32 0), ptr @var |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps rename var to something else then add
store ptr ptrauth (ptr @func, i32 0), ptr @g
store ptr ptrauth (ptr @func_undef, i32 0), ptr @g
store ptr ptrauth (ptr @var_undef, i32 0), ptr @g
store ptr ptrauth (ptr @var, i32 0), ptr @g
Use llvm.used so that the symbols will not be optimized out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use llvm.used so that the symbols will not be optimized out.
Maybe I'm doing smth wrong, but it looks like that llvm.used has no effect and only the symbol from the last store operation is present in GOT. This is what I've added to the code (right before @bar definition, just in case):
@llvm.used = appending global [4 x ptr] [ ptr @func, ptr @func_undef, ptr @var, ptr @var_undef], section "llvm.metadata"
Adding @g to this array, removing section "llvm.metadata", moving this array definition to the end of the file does not change the behavior.
This smells like a bug, given llvm.used description from IR reference: https://llvm.org/docs/LangRef.html#the-llvm-used-global-variable.
@MaskRay does this look like a bug for you as well, or maybe I'm just mis-using this special array?
As for now, I've just used a separate global for each store - g1, g2, g3, g4. Not elegant, but at least it works :)
@MaskRay Do I get your intention correct - you suggest to change this: to smth like this: If so, this does not look correct - function symbols from Please let me know if I misunderstood you. |
I've added a check against AArch64 - see efaa9e2. As for check against PAuth - probably we can look through relocations, and if at least one AUTH GOT-generating relocation is present ( @MaskRay Please let me know if such a check for PAuth looks OK for you. Probably, in future we could rely on build attributes section, but, as far as I understand, this is not currently supported in lld (please correct me if I'm wrong). |
…#131467) Symtab is first filled with the data from the bitcode file, and all undefined symbols except TLS ones are `STT_NOTYPE`. Since auth key for a signed GOT entry depends on the symbol type being `STT_FUNC` or not, we need to update the symtab after the bitcode is compiled to an ELF object and update symbol types for function symbols. This patch implements the described behavior.
Symtab is first filled with the data from the bitcode file, and all undefined symbols except TLS ones are
STT_NOTYPE. Since auth key for a signed GOT entry depends on the symbol type beingSTT_FUNCor not, we need to update the symtab after the bitcode is compiled to an ELF object and update symbol types for function symbols. This patch implements the described behavior.